Skip to content

0.27.3 - #2062

Merged
alex-zaitsev merged 50 commits into
masterfrom
0.27.3
Aug 12, 2026
Merged

0.27.3#2062
alex-zaitsev merged 50 commits into
masterfrom
0.27.3

Conversation

@sunsingerus

@sunsingerus sunsingerus commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

aaron276h and others added 30 commits July 2, 2026 20:49
A single transient apiserver/network error (connection refused/reset,
timeout, 5xx) during a CHI reconcile currently propagates up as-is and
aborts the entire reconcile, discarding progress on all remaining
hosts. On installations with hundreds of hosts a reconcile makes tens
of thousands of API calls over a long wall-clock window, so even a
sub-second control-plane blip is likely to hit one of them and abort
an otherwise healthy rollout, leaving the CR stuck in Aborted state.

Wrap all Get calls in the chi kube drivers with a bounded
exponential-backoff retry (5 attempts, 500ms..4s doubling, ~7.5s total
budget) that retries only transient network/API errors. Terminal
errors (NotFound, AlreadyExists, Conflict, Forbidden, Unauthorized,
BadRequest, Invalid, MethodNotSupported) and context cancellation
surface immediately, and when retries are exhausted the last error is
returned unchanged - so callers behave exactly as before for
non-transient errors and sustained outages.

Signed-off-by: Aaron Harlap <aaron@chronosphere.io>
Update the race-only test to use the current metav1.Object-based Registry API.

Signed-off-by: Rohan Thakkar <rohant@twitter.com>
Decode direct objects and DeletedFinalStateUnknown tombstones through one typed
delete handler, reporting malformed events instead of panicking.

Signed-off-by: Rohan Thakkar <rohant@twitter.com>
… calls

Integrates aaron276h's PR #2025 (github.com//pull/2025, fixes #2026):
bounded exponential-backoff retry of transient K8s API/network errors on the chi kube
driver Get calls, so a sub-second control-plane blip no longer aborts an entire
multi-hundred-host reconcile. Follow-up commit refactors the helper into
controller/common/kube (shared with CHK), makes the back-off ctx-cancellable, and
tightens the net.Error classifier.
…nown tombstones

Routes all CHI-controller informer DeleteFuncs through generic
deletedObject[T]/deleteHandler[T] helpers so client-go tombstone
payloads (missed-watch relist) are decoded instead of panicking.
Fixes #1882, #671.
- retire dead handleObject stub + messageUnableToDecode const (only
  callers were dead comments; deletedObject supersedes its tombstone
  decode branch)
- document why deletedObject uses value-type T (typed-nil guard)
- rename TestDeletedObject -> Test_deletedObject for sibling consistency
serviceMonitor.enabled now also creates a separate ServiceMonitor that
scrapes native ClickHouse Keeper prometheus endpoints from every keeper
service managed by the operator (matched by
clickhouse-keeper.altinity.com/app=chop label, port name `metrics`).
Opt-out via serviceMonitor.keeperMetrics.enabled=false.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
sunsingerus and others added 19 commits August 1, 2026 13:58
Reworks PR #2046 (issue #1704) on three counts found by review:

- A deferred host returned ErrCRUDAbort, which the shard[0] early-return turned
  into 'skip every sibling shard', and also skipped clean(), monitoring and the
  ancestor update. Deferral now has its own sentinel, is accumulated across shards
  and clusters, and is surfaced once at the end of the pass. It deliberately still
  reaches the top as an error: a pass that swallowed it would advance the ancestor
  past a host that was never rolled and let clean() purge that host.

- hostMayRequireDisruption tested !Is(ObjectStatusSame), but at that point host
  statuses are only Requested/Modified/Found - Same is assigned later, inside
  ReconcileStatefulSet. Every converged host therefore looked disruptable, so a
  healthy replica in a degraded shard got no reconcile at all. Replaced by
  hostDisruptionWouldDegradeShard, evaluated in reconcileHostStatefulSet after
  PrepareHostStatefulSetWithStatus where the status is authoritative.

- runConcurrently was last-writer-wins, so a benign deferral could mask a real
  shard failure. Deferrals are now tracked apart from errors.

shouldExcludeHost keeps a shard-safety check, now for the opposite reason: since
the restart is deferred later in the same pass, draining the host here would strip
its ready label and drop it from the Service endpoints with nothing left to put it
back - emptying the entrypoint Service of a shard whose only other replica is down.

Deferral is surfaced via ReconcileAbortWithReason plus a dedicated event reason so
'Aborted' names its cause.
test_010083 covers one shard, so the deferral fix itself was untested: nothing
proved that a host deferred for shard safety still lets the other shards update.

Adds test_010083_1 - 2 shards x 2 replicas with shard 0 replica 0 parked on a
broken image, so shard 0 replica 1 is its shard's last healthy replica. Rolling a
new image to everything else must:

  - converge shard 1                        (the fix; a deferral used to abort the pass)
  - leave shard 0 replica 1 serving         (the protection test_010083 already covers)
  - still report Aborted                    (the deferral is surfaced, not swallowed)
  - NOT advance status-normalizedCompleted  (else the pending roll is diffed away and lost)
  - NOT purge the deferred host's STS       (else clean() deletes the replica being protected)

Verified both ways: fails against the unreworked PR on exactly the sibling
convergence assertion, passes with the rework.

Known gap: the roll is driven by an image change, which leaves shouldForceRestartHost
false, so the exclude/drain path is not exercised here.
…tarving sibling shards

Brings in PR #2046 (issue #1704 - an interrupted roll could take down a shard's
last healthy replica) together with a rework of it.

The deferral now has its own sentinel and is accumulated across shards and
clusters, so one deferred host no longer skips every sibling shard. It still
reaches the top as an error on purpose: swallowing it would advance the ancestor
past a host that was never rolled and let clean() purge that host's StatefulSet.

The disruption gate moved into reconcileHostStatefulSet, after the desired
StatefulSet is known - the previous predicate tested a status that is only
assigned later, so every converged host looked disruptable and a healthy replica
in a degraded shard received no reconcile at all. shouldExcludeHost keeps its
shard-safety check so a host that is about to be deferred is not drained out of
the Service with nothing left in the pass to restore it.

Tests: unit coverage for the shard-safety predicate and the sentinel, plus e2e
test_010083_1, verified to fail against the unreworked PR on exactly the sibling
convergence assertion.
70-chop-config.yaml is described as "a fully-annotated example" by nine
generated manifests, but it covers roughly 60% of the options the chopconf
CRD declares and has never been the exhaustive reference those comments
imply. It also ships: operatorhub.sh loads it into the OLM CSV alm-examples
annotation, which is the YAML an OpenShift user gets pre-filled on
"Create ClickHouseOperatorConfiguration" - a role that wants a short sample,
not a complete catalogue.

Adds 99-clickhouseoperatorconfiguration-max.yaml as the counterpart of
99-clickhouseinstallation-max.yaml, covering 81 of the 95 leaves the chopconf
CRD declares, and points the docs at it for the full option list while
70-chop-config.yaml stays the short starting point.

The 14 uncovered leaves are the reconcile.host.hooks fields, shown commented
out: the chopconf CRD omits their `events` and `failurePolicy` sub-fields,
so a hook set through the CR is accepted, stored and never fires.

The file documents the merge rules up front, since they are easy to get wrong:
explicitly-set values pin over future defaults, list-valued settings append
rather than replace, and clickhouse.metrics.excludeRegexp is the one exception.
Document Go dependency licenses, add Apache NOTICE, and surface license metadata in README, CONTRIBUTING, and the Helm chart.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add third-party notices and clarify license attribution
@alex-zaitsev
alex-zaitsev merged commit 41b4f28 into master Aug 12, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants